Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add ci for test, lint, format and typecheck #72

Merged
merged 5 commits into from
Jan 17, 2024
Merged

fix: add ci for test, lint, format and typecheck #72

merged 5 commits into from
Jan 17, 2024

Conversation

lstocchi
Copy link
Contributor

No description provided.

@lstocchi lstocchi force-pushed the i38 branch 2 times, most recently from 1eee17a to 96f7fb5 Compare January 17, 2024 10:44
@slemeur
Copy link
Contributor

slemeur commented Jan 17, 2024

🚀

@lstocchi lstocchi linked an issue Jan 17, 2024 that may be closed by this pull request
@feloy
Copy link
Contributor

feloy commented Jan 17, 2024

It seems you will have to run this command locally, to get some files correctly formatted (beware, files will be modified in place):

./node_modules/.bin/prettier -w --check "**/src/**/*.ts"

benoitf and others added 3 commits January 17, 2024 13:53
shared package has an issue.
shared module should not depends on 'web' library
because backend importing these classes is a pure Node.JS module

so some classes of shared should be only on frontend

fix a test returning a string and not a promise
chore: some workaround for podman-desktop/api
@lstocchi lstocchi marked this pull request as ready for review January 17, 2024 14:10
.github/workflows/pr-check.yaml Outdated Show resolved Hide resolved
.github/workflows/pr-check.yaml Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
"svelte": "4.2.8",
"svelte-preprocess": "^5.1.3",
"tailwindcss": "^3.4.0",
"postcss": "^8.4.33",
"postcss-load-config": "^5.0.2"
"postcss-load-config": "^5.0.2"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it looks like the file is not formatted/sorted (usually it is ordered in alphabetical order) and there is an extra space here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how do you sort it automatically? Is there any npm package? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is done when you run yarn add <package>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was hoping we could add some script so that if someone copy/paste an entry it is sorted anyway

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

npx sort-package-json

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(anyway it's not a requested change)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool, i'm going to add it in a follow-up PR so i don't block @feloy
for the moment i fixed it manually

Signed-off-by: lstocchi <[email protected]>
@lstocchi lstocchi merged commit 874f138 into main Jan 17, 2024
3 checks passed
@lstocchi lstocchi deleted the i38 branch January 17, 2024 15:27
mhdawson pushed a commit to mhdawson/podman-desktop-extension-ai-lab that referenced this pull request Nov 22, 2024
* add option for config file in playground

* Update playground/README.md

Co-authored-by: Sébastien Han <[email protected]>

---------

Co-authored-by: Sébastien Han <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ci: run tests as part of the CI
4 participants